Skip to content

fix: put a timeout to callers of methods that use singleflight#3048

Draft
miparnisari wants to merge 1 commit intomainfrom
deadlock-singleflight
Draft

fix: put a timeout to callers of methods that use singleflight#3048
miparnisari wants to merge 1 commit intomainfrom
deadlock-singleflight

Conversation

@miparnisari
Copy link
Copy Markdown
Contributor

@miparnisari miparnisari commented Apr 15, 2026

Description

Potential fix for #2788

image

TO DO: discuss with @josephschorr. I want to overhaul the singleflight code (specifically singleflight.go, optimized.go, and context.go).

  • move the code of optimized.go into singleflight.go
  • move CRDB's featureGroup into singleflight.go
image

Testing

docker-compose -f docker-compose.deadlock.yaml up -d
sh reproduce_deadlock.sh

reproduce_deadlock.sh
docker-compose.deadlock.yaml

References

@github-actions github-actions Bot added the area/datastore Affects the storage system label Apr 15, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.57%. Comparing base (1df1014) to head (a0c6c4e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/datastore/proxy/singleflight.go 86.67% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3048      +/-   ##
==========================================
+ Coverage   75.51%   75.57%   +0.06%     
==========================================
  Files         502      502              
  Lines       61769    61805      +36     
==========================================
+ Hits        46638    46702      +64     
+ Misses      11716    11699      -17     
+ Partials     3415     3404      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@miparnisari miparnisari force-pushed the deadlock-singleflight branch 2 times, most recently from 38517ea to 866fb65 Compare April 20, 2026 21:05
@miparnisari miparnisari force-pushed the deadlock-singleflight branch 2 times, most recently from 45d61a2 to d6f53ed Compare May 2, 2026 00:55
@github-actions github-actions Bot added area/cli Affects the command line area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels May 2, 2026
@miparnisari miparnisari changed the title fix: avoid deadlocking singleflighted reqs fix: put a timeout to callers of methods that use singleflight May 7, 2026
Comment on lines -546 to -548
features, _, err := cds.featuresGroup.Do(ctx, "", func(ictx context.Context) (*datastore.Features, error) {
return cds.features(ictx)
})
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI this is a refactor - i dislike having singleflight uses scattered in many places.

@miparnisari miparnisari force-pushed the deadlock-singleflight branch 3 times, most recently from 1c6f606 to efb210f Compare May 7, 2026 20:26
@miparnisari miparnisari force-pushed the deadlock-singleflight branch from efb210f to a0c6c4e Compare May 7, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli Affects the command line area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant